Skip to content

perf: Combat perf improvements#4070

Merged
flying-sheep merged 11 commits into
scverse:mainfrom
ilaykav:combat-perf-improvements
Apr 24, 2026
Merged

perf: Combat perf improvements#4070
flying-sheep merged 11 commits into
scverse:mainfrom
ilaykav:combat-perf-improvements

Conversation

@ilaykav
Copy link
Copy Markdown
Contributor

@ilaykav ilaykav commented Apr 17, 2026 •
edited by flying-sheep
Loading

  • Closes # - Perf Improvement
  • Tests included or not required because: all tests pass, Added combat benchmark
  • Release notes not necessary because: internal perf improvement

Devloped on venv, ran benchamrk locally, main changes:

  • Replaced np.dot(x, np.ones(...)) with numpy broadcasting ([:, np.newaxis])
  • Extract .values from DataFrames before computation loops
    Benchmarked locally on 50K cells × 500 genes, 2-3 speedup on pp.combat

@ilaykav ilaykav changed the title fix: Combat perf improvements perf: Combat perf improvements Apr 17, 2026
@ilaykav ilaykav force-pushed the combat-perf-improvements branch from 7b601bf to c0e36aa Compare April 17, 2026 14:37
@flying-sheep flying-sheep added this to the 1.12.2 milestone Apr 17, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.59%. Comparing base (d15aede) to head (f994269).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4070      +/-   ##
==========================================
- Coverage   78.60%   78.59%   -0.02%     
==========================================
  Files         118      118              
  Lines       12753    12745       -8     
==========================================
- Hits        10025    10017       -8     
  Misses       2728     2728
Flag Coverage Δ
hatch-test.low-vers 77.89% <100.00%> (-0.02%) ⬇️
hatch-test.pre 78.47% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/scanpy/preprocessing/_combat.py 88.18% <100.00%> (-0.70%) ⬇️

@ilaykav
Copy link
Copy Markdown
Contributor Author

ilaykav commented Apr 17, 2026 •
edited
Loading

~1.5x-2x times faster on 3K-20K cells, measured locally on my Intel i7-8665U python 3.14

flying-sheep
flying-sheep requested changes Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@flying-sheep flying-sheep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, thanks! there are a few superficial issues.

About the general idea, why do you think that to_numpy() influences anything? For pandas arrays that already wrap underlying numpy arrays, the method should have basically no performance impact.

Comment thread benchmarks/benchmarks/tools.py Outdated
Comment thread benchmarks/benchmarks/tools.py Outdated
Comment thread src/scanpy/preprocessing/_combat.py Outdated
Comment thread src/scanpy/preprocessing/_combat.py Outdated
@scverse-benchmark
Copy link
Copy Markdown

scverse-benchmark Bot commented Apr 23, 2026 •
edited
Loading

Benchmark changes

Change Before [d15aede] After [d94816f] Ratio Benchmark (Parameter)
- 495±30μs 389±3μs 0.79 preprocessing_counts.FastSuite.time_log1p('pbmc68k_reduced', 'counts-off-axis')
- 110±0.8ms 76.3±1ms 0.7 preprocessing_log.CombatSuite.time_combat

Comparison: https://github.com/scverse/scanpy/compare/d15aedeb0300d04bb327333e30bebfc0aa37d87c..d94816fc8f252ffa9a4f919aaacc80dbbf505eb4
Last changed: Fri, 24 Apr 2026 08:00:37 +0000

More details: https://github.com/scverse/scanpy/pull/4070/checks?check_run_id=72836821961

@ilaykav
Copy link
Copy Markdown
Contributor Author

ilaykav commented Apr 23, 2026

You're right, the broadcasting itself (and the memory save from removing the np.ones allocations, not the .to_numpy()) is what actually made the impact, dropping the .to_numpy() change

@ilaykav ilaykav force-pushed the combat-perf-improvements branch from b15cf74 to d94816f Compare April 23, 2026 23:39
flying-sheep and others added 3 commits April 24, 2026 09:29
@flying-sheep
inline
20f686c
@flying-sheep
bring back some .to_numpy
9e80853 
Co-authored-by: Copilot <copilot@github.com>
@flying-sheep
move suite
f994269
@flying-sheep flying-sheep merged commit d6f2c51 into scverse:main Apr 24, 2026
14 checks passed
@flying-sheep
Copy link
Copy Markdown
Member

flying-sheep commented Apr 24, 2026

thank you!

@meeseeksmachine meeseeksmachine mentioned this pull request Apr 24, 2026
Merged
flying-sheep pushed a commit that referenced this pull request Apr 24, 2026
@meeseeksmachine @ilaykav
Backport PR #4070 on branch 1.12.x (perf: Combat perf improvements) (#…
746a05b 
…4085)

Co-authored-by: Ilay Kavitzky <ilay.kavitzky@gmail.com>
flying-sheep added a commit that referenced this pull request Apr 26, 2026
@ilaykav @pre-commit-ci @flying-sheep
perf: Combat perf improvements (#4070)
6b7c1d1 
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Philipp A. <flying-sheep@web.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@flying-sheep flying-sheep flying-sheep approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.12.2

Development

Successfully merging this pull request may close these issues.

2 participants

@ilaykav @flying-sheep